Add Config.SoftStopTimeout for a cleaner way to shut a client down#1239
Conversation
c52de36 to
b7f599c
Compare
d5d0c0b to
cc36ec9
Compare
bgentry
left a comment
There was a problem hiding this comment.
Looks great! My only question is whether the more advanced version (previous) should be preserved in documentation or examples somewhere, even though most people won't need it. I don't think it's essential though, up to you.
A part of River code that's always bothered me is the graceful shutdown
example [1]. It's really, really complicated, and I think that
complicated orchestration is very likely a sign that our code isn't
quite right.
I initially tried putting the graceful shutdown code into a helper like
`river.GracefulShutdown`, but I found that also problematic because it's
not super clean to call, and even minor customizations would require a
user to copy out the entire function.
Playing with an LLM a bit, I came up with this alternative: build an
understanding of soft/hard stop into the client's default `Stop`
function by adding `Config.SoftStopTimeout`. By default, leaving this
value unconfigured keeps the existing behavior of River today where a
call to `Stop` waits unbounded time for jobs to finish working. Adding a
`SoftStopTimeout` value brings in functionality similar to the graceful
shutdown example: stopping proceeds normally waiting for jobs to finish,
but in case they don't inside of `SoftStopTimeout`, they're cancelled as
if `StopAndCancel` had been called.
The soft timeout is also respected for a cancelled `Start` context,
which makes the basic use of River for soft/hard stop very nice (you
don't even need to call `Stop` anymore):
// Use signal.NotifyContext to cancel the start context on SIGINT/SIGTERM.
// When the signal fires, the client initiates a soft stop. If running jobs
// don't finish within SoftStopTimeout, their contexts are automatically
// cancelled.
ctx, stop := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM)
defer stop()
if err := riverClient.Start(ctx); err != nil {
panic(err)
}
[1] https://github.com/riverqueue/river/blob/master/example_graceful_shutdown_test.go
cc36ec9 to
80820ee
Compare
Thanks! I added back a The only downside is that these example tests do tend to add a little overhead to the test suite since they all have to run sequentially, and many of them are doing slow-ish things involving River client startup and stop. If this is starting to get bad, we may want to take this new one back out again as it may only be of marginal use overall. |
Here, prepare release v0.38.0, containing the changes in #1239 that add `Config.SoftStopTimeout`. [skip ci]
Here, prepare release v0.38.0, containing the changes in #1239 that add `Config.SoftStopTimeout`. [skip ci]
A part of River code that's always bothered me is the graceful shutdown
example [1]. It's really, really complicated, and I think that
complicated orchestration is very likely a sign that our code isn't
quite right.
I initially tried putting the graceful shutdown code into a helper like
river.GracefulShutdown, but I found that also problematic because it'snot super clean to call, and even minor customizations would require a
user to copy out the entire function.
Playing with an LLM a bit, I came up with this alternative: build an
understanding of soft/hard stop into the client's default
Stopfunction by adding
Config.SoftStopTimeout. By default, leaving thisvalue unconfigured keeps the existing behavior of River today where a
call to
Stopwaits unbounded time for jobs to finish working. Adding aSoftStopTimeoutvalue brings in functionality similar to the gracefulshutdown example: stopping proceeds normally waiting for jobs to finish,
but in case they don't inside of
SoftStopTimeout, they're cancelled asif
StopAndCancelhad been called.The soft timeout is also respected for a cancelled
Startcontext,which makes the basic use of River for soft/hard stop very nice (you
don't even need to call
Stopanymore):[1] https://github.com/riverqueue/river/blob/master/example_graceful_shutdown_test.go